Additional Option binding updates by alzimmermsft · Pull Request #2922 · microsoft/mcp

alzimmermsft · 2026-06-22T16:12:37Z

What does this PR do?

Splits out complex type handling to [OptionContainer] attribute so [Option] doesn't perform double duty.
Now that [Option] doesn't need to support complex type handling, remove constructors and mark Description as a required property. Updated guidance and source code accordingly.
Added back empty or whitespace checking on required string values without a default value supplier.
Added support for AllowEmptyOrWhiteSpaceString in [Option] to allow required strings to support specialty cases.
Updated binding to account for errors in parsing results. Removed special casing for missing parameters being the response message if they exist as this can mask additional parsing errors (ex, had missing options and an invalid enum value).

GitHub issue number?

[Link to the GitHub issue this PR addresses]

Pre-merge Checklist

…whitespace string validation

Copilot

Pull request overview

This PR updates the shared option-binding system (and many downstream tool option POCOs) to separate complex/nested option handling into a new [OptionContainer] attribute, make [Option].Description required, and improve validation/error reporting during binding.

Changes:

Introduces [OptionContainer] for complex/nested option groups (e.g., retry policy), and removes complex-type responsibilities from [Option].
Makes [Option].Description required and updates option definitions across multiple toolsets accordingly.
Improves binding-time error reporting (includes parse errors) and adds required-string empty/whitespace validation with an escape hatch (AllowEmptyOrWhiteSpaceString), plus adjusts impacted tests/docs.

Reviewed changes

Copilot reviewed 74 out of 74 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tools/Fabric.Mcp.Tools.OneLake/src/Options/WorkspaceListOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/TableNamespaceListOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/TableNamespaceGetOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/TableListOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/TableGetOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/TableConfigGetOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/PathListOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Options/BlobListOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemListDfsCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemListCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemDataListCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileWriteCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileReadCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileDeleteCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/DirectoryDeleteCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/DirectoryCreateCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobPutCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobGetCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobDeleteCommand.cs	Updates embedded options type to use required `Description = ...`.
tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetWorkloadDefinitionCommandTests.cs	Loosens assertion to tolerate additional parse/bind error text.
tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetWorkloadApisCommandTests.cs	Loosens assertion to tolerate additional parse/bind error text.
tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetExamplesCommandTests.cs	Loosens assertion to tolerate additional parse/bind error text.
tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetBestPracticesCommandTests.cs	Loosens assertion to tolerate additional parse/bind error text.
tools/Fabric.Mcp.Tools.Docs/src/Options/PublicApis/WorkloadCommandOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.Docs/src/Options/BestPractices/GetBestPracticesOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/RunPipelineOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/ListPipelinesOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/GetPipelineOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/CreatePipelineOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Dataflow/ListDataflowsOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Dataflow/ExecuteQueryOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.DataFactory/src/Options/Dataflow/CreateDataflowOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Fabric.Mcp.Tools.Core/src/Options/ItemCreateOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Azure.Mcp.Tools.Storage/src/Options/Table/TableListOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Storage/src/Options/Blob/Container/ContainerGetOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Storage/src/Options/Blob/Container/ContainerCreateOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Storage/src/Options/Blob/BlobUploadOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Storage/src/Options/Blob/BlobGetOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Storage/src/Options/Account/AccountGetOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Storage/src/Options/Account/AccountCreateOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Kusto/tests/Azure.Mcp.Tools.Kusto.Tests/QueryCommandTests.cs	Removes unused test using import.
tools/Azure.Mcp.Tools.Kusto/src/Options/TableSchemaOptions.cs	Standardizes option attributes and cluster option naming (`Cluster`).
tools/Azure.Mcp.Tools.Kusto/src/Options/TableListOptions.cs	Standardizes option attributes and cluster option naming (`Cluster`).
tools/Azure.Mcp.Tools.Kusto/src/Options/SampleOptions.cs	Standardizes option attributes and cluster option naming (`Cluster`).
tools/Azure.Mcp.Tools.Kusto/src/Options/QueryOptions.cs	Standardizes option attributes and cluster option naming (`Cluster`).
tools/Azure.Mcp.Tools.Kusto/src/Options/IClusterOption.cs	Renames interface member to `Cluster` to match option naming.
tools/Azure.Mcp.Tools.Kusto/src/Options/DatabaseListOptions.cs	Standardizes option attributes and cluster option naming (`Cluster`).
tools/Azure.Mcp.Tools.Kusto/src/Options/ClusterListOptions.cs	Standardizes option attributes and retry container handling.
tools/Azure.Mcp.Tools.Kusto/src/Options/ClusterGetOptions.cs	Standardizes option attributes and required cluster option naming (`Cluster`).
tools/Azure.Mcp.Tools.Kusto/src/Commands/TableSchemaCommand.cs	Updates option property reference from `ClusterName` to `Cluster`.
tools/Azure.Mcp.Tools.Kusto/src/Commands/TableListCommand.cs	Updates option property reference from `ClusterName` to `Cluster`.
tools/Azure.Mcp.Tools.Kusto/src/Commands/SampleCommand.cs	Updates option property reference from `ClusterName` to `Cluster`.
tools/Azure.Mcp.Tools.Kusto/src/Commands/QueryCommand.cs	Updates option property reference from `ClusterName` to `Cluster`.
tools/Azure.Mcp.Tools.Kusto/src/Commands/DatabaseListCommand.cs	Updates option property reference from `ClusterName` to `Cluster`.
tools/Azure.Mcp.Tools.Kusto/src/Commands/ClusterGetCommand.cs	Updates option property reference from `ClusterName` to `Cluster`.
tools/Azure.Mcp.Tools.Kusto/src/Commands/BaseClusterCommand.cs	Updates validation to require `Cluster` instead of `ClusterName`.
tools/Azure.Mcp.Tools.ApplicationInsights/src/Options/RecommendationListOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.AppLens/src/Options/Resource/ResourceDiagnoseOptions.cs	Updates `[Option]` usage to require `Description = ...`.
tools/Azure.Mcp.Tools.AppConfig/src/Options/KeyValue/Lock/KeyValueLockSetOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Aks/src/Options/Nodepool/NodepoolGetOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Aks/src/Options/Cluster/ClusterGetOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Acr/src/Options/Registry/RegistryRepositoryListOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
tools/Azure.Mcp.Tools.Acr/src/Options/Registry/RegistryListOptions.cs	Switches nested retry options to `[OptionContainer]` and updates descriptions.
servers/Azure.Mcp.Server/docs/new-command.md	Updates option authoring guidance/examples for required `Description` and containers.
docs/option-conversion.md	Updates option conversion guidance/examples for required `Description` and containers.
core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/RecordedCommandTestsBase.cs	Refactors live-test-only skipping to shared helper.
core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs	Ensures settings are loaded before live-test-only checks; adds helper.
core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.Tests/Options/OptionBinderTests.cs	Updates tests for required descriptions; adds new validation tests.
core/Microsoft.Mcp.Core/src/Options/OptionTypeHandler.cs	Adds empty/whitespace validation and routes string/enum binding accordingly.
core/Microsoft.Mcp.Core/src/Options/OptionDescriptor.cs	Adds `[OptionContainer]` support and enforces attribute correctness for complex types.
core/Microsoft.Mcp.Core/src/Options/OptionContainerAttribute.cs	Adds new attribute to mark complex/nested option containers.
core/Microsoft.Mcp.Core/src/Options/OptionBinder.cs	Includes parse errors in binding failures and simplifies validation exception creation.
core/Microsoft.Mcp.Core/src/Options/OptionAttribute.cs	Removes constructors and makes `Description` required; adds empty/whitespace allow flag.
core/Microsoft.Mcp.Core/src/Commands/BaseCommand`2.cs	Uses `CommandValidationException.Message` directly to avoid masking parser errors.

vcolin7 · 2026-06-22T18:35:45Z


            throw new CommandValidationException(
                string.Join('\n', messages),
-                HttpStatusCode.BadRequest,


Which status code do we now bubble up to the root JSON object?

vcolin7 · 2026-06-22T21:36:52Z

 - **Name**: Derived automatically from the property name in kebab-case (e.g., `LocalFilePath` → `--local-file-path`). Only use `[Option(Name = "...")]` when the convention doesn't produce the desired name (e.g., `RetryPolicy` → `--retry` instead of `--retry-policy`). **Do not** specify `Name =` when it matches the default.
- **Required**: Determined by nullability. Use `required` keyword or non-nullable types for required options. Use `?` for optional.
- **Description**: Pass as the constructor argument `[Option("description")]` or via `[Option(Description = "...")]`.
+- **Required**: Driven by the `required` keyword (`RequiredMemberAttribute`). Use `required` on required options; use nullable types (`?`) for optional options.


What is RequiredMemberAttribute? Is it one of our core classes?

vcolin7 · 2026-06-22T21:44:41Z

-    // TODO: Remove unused option — registered and visible to the user but never consumed by the command
-    [Option(OptionDescriptions.AuthMethod)]
-    public AuthMethod? AuthMethod { get; set; }


We should mention this in the changelog

vcolin7 · 2026-06-22T21:45:05Z

-    // TODO: Remove unused option — registered and visible to the user but never consumed by the command
-    [Option(OptionDescriptions.AuthMethod)]
-    public AuthMethod? AuthMethod { get; set; }


Let's mention user-facing changes in the changelog, even if these options were actually unused :)

vcolin7 · 2026-06-22T21:45:15Z

-    // TODO: Remove unused option — registered and visible to the user but never consumed by the command
-    [Option(OptionDescriptions.AuthMethod)]
-    public AuthMethod? AuthMethod { get; set; }


Mention in changelog

vcolin7 · 2026-06-22T21:51:08Z

                databasesNames = await kustoService.ListDatabasesAsync(
                    options.Subscription!,
-                    options.ClusterName!,
+                    options.Cluster!,


Based on the previous PR I assumed we'd shift these checks to the *Options class by using the required keyword. I am happy doing it on both the *Options and *Command classes, but I think we should write a guideline and keep consistency across the repo.

vcolin7 · 2026-06-22T21:51:26Z

                results = await kustoService.QueryItemsAsync(
                    options.Subscription!,
-                    options.ClusterName!,
+                    options.Cluster!,


Same comment on requiredness.

vcolin7 · 2026-06-22T21:51:30Z

                results = await kustoService.QueryItemsAsync(
                    options.Subscription!,
-                    options.ClusterName!,
+                    options.Cluster!,


Same comment on requiredness.

vcolin7 · 2026-06-22T21:51:35Z

                tableNames = await kustoService.ListTablesAsync(
                    options.Subscription!,
-                    options.ClusterName!,
+                    options.Cluster!,


Same comment on requiredness.

vcolin7 · 2026-06-22T21:51:38Z

                tableSchema = await kustoService.GetTableSchemaAsync(
                    options.Subscription!,
-                    options.ClusterName!,
+                    options.Cluster!,


Same comment on requiredness.

Add OptionContainer, require Description in Option, support empty or …

adac45f

…whitespace string validation

alzimmermsft self-assigned this Jun 22, 2026

Copilot AI review requested due to automatic review settings June 22, 2026 16:12

alzimmermsft requested review from a team, danield137, jongio, msalaman, prvavill, xiangyan99 and xiaomi7732 as code owners June 22, 2026 16:12

github-project-automation Bot added this to Azure MCP Server Jun 22, 2026

alzimmermsft requested review from JonathanCrd, conniey and gossion as code owners June 22, 2026 16:12

alzimmermsft requested review from chidozieononiwu, hallipr and joshfree June 22, 2026 16:12

github-project-automation Bot moved this to Untriaged in Azure MCP Server Jun 22, 2026

github-actions Bot added tools-ACR tools-Aks tools-AppConfig AppConfig tools-AppLens tools-ApplicationInsights Application Insights tools-Kusto tools-Storage labels Jun 22, 2026

Copilot started reviewing on behalf of alzimmermsft June 22, 2026 16:16 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

alzimmermsft added 2 commits June 22, 2026 12:28

PR feedback

0fdd572

Merge branch 'main' into OptionBindingAdditionalFollowups

b7dc55a

Fix a test

d9f9b70

vcolin7 reviewed Jun 22, 2026

View reviewed changes

Conversation

alzimmermsft commented Jun 22, 2026

What does this PR do?

GitHub issue number?

Pre-merge Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants